Skip to content

feat: Support dashboard variables in chart builder tiles - #2901

Merged
pulpdrew merged 4 commits into
mainfrom
drew/builder-variables
Aug 17, 2026
Merged

feat: Support dashboard variables in chart builder tiles#2901
pulpdrew merged 4 commits into
mainfrom
drew/builder-variables

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This PR extends dashboard variable support to Builder charts.

  • Variables are substituted in most user-editable chart-builder fields (where, aggCondition, group by, having, order by, etc). This works for both SQL and Lucene. Lucene does not get any macros, and SQL only gets variable-related macros (not the full suite available in Raw SQL charts).
  • Variables and (when available) macros are suggested in autocomplete in both SQL and Lucene inputs, when available.
  • Warnings are shown on the right side of the input when a variable is referenced but not found (as well as some other validations).

Note that creation of variables is gated behind NEXT_PUBLIC_ENABLE_DASHBOARD_VARIABLES (enabled by default in development locally.

Future work

Future work includes, but is not limited to:

  1. Updating the alerts runner to supply a variables context based on the variables configured on the dashboard
  2. Supporting dependent variables
  3. MCP and external API support
  4. Documentation

Screenshots or video

Screen.Recording.2026-08-13.at.12.21.41.PM.mov

How to test locally

  1. Clone and restart to ensure NEXT_PUBLIC_ENABLE_DASHBOARD_VARIABLES=true
  2. Create a dashboard
  3. Add a filter and enable variable mode
  4. Add a builder tile and test the replacement, autocomplete, and validation functionality. Try SQL and lucene, bare references and macros, in various inputs (not just WHERE!)

References

  • Linear Issue: Closes HDX-5056 HDX-5058
  • Related PRs:

@vercel

vercel Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 17, 2026 5:58pm
hyperdx-storybook Ready Ready Preview Aug 17, 2026 5:58pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 60025e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@hyperdx/app Patch
@hyperdx/common-utils Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Base automatically changed from drew/sql-variable-validations to main August 13, 2026 14:49
@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from 5531572 to 9df95f9 Compare August 13, 2026 14:49
@github-actions

github-actions Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 299 passed • 1 skipped • 1134s

Status Count
✅ Passed 299
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@greptile-apps

greptile-apps Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends dashboard-variable support from raw SQL charts to chart-builder tiles.

  • Substitutes variables across builder expressions before chart-query rendering.
  • Adds SQL and Lucene variable autocomplete and reference validation.
  • Passes referenced dashboard variables into builder tile previews and runtime queries.
  • Adds unit and end-to-end coverage for variable-enabled chart editing.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed
Filename Overview
packages/common-utils/src/variables.ts Adds shared token scanning, reference validation, language-aware builder-field traversal, and chart-config variable substitution.
packages/common-utils/src/core/renderChartConfig.ts Substitutes dashboard variables before the existing builder chart rendering pipeline.
packages/app/src/DBDashboardPage.tsx Supplies referenced dashboard variables to builder tiles and provides editor-wide variable completions.
packages/app/src/components/SQLEditor/variableCompletions.tsx Centralizes variable and variable-macro completion generation and React context propagation.
packages/app/src/components/SQLEditor/variableValidation.tsx Adds non-blocking validation indicators for variable references in expression editors.
packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx Integrates dashboard variables into builder preview configuration and editor behavior.
packages/app/tests/e2e/features/dashboard.spec.ts Exercises dashboard variable substitution, autocomplete, validation, and builder-tile behavior.

Flowchart

 
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Filters[Dashboard filters and variable selections] --> Context[Chart variable context]
  Context --> Editor[Builder expression editors]
  Context --> Tile[Saved builder tile]
  Editor --> Completion[Autocomplete and validation]
  Tile --> References[Referenced-variable filtering]
  References --> Config[Builder chart config with variables]
  Config --> Substitute[Language-aware variable substitution]
  Substitute --> Render[Chart config SQL rendering]
  Render --> ClickHouse[(ClickHouse)]
Loading

Reviews (7): Last reviewed commit: "fix: Fix sample matching events variable..." | Re-trigger Greptile

@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from 9df95f9 to 9e8fb64 Compare August 13, 2026 15:28
@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from 9e8fb64 to b94023d Compare August 13, 2026 15:45
pulpdrew
pulpdrew commented Aug 13, 2026
View reviewed changes
Comment thread packages/common-utils/src/variables.ts
case 'lucene':
return values.length === 0
? '*'
? '("")'

@pulpdrew pulpdrew Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a no-op (1=1), whereas * is a notEmpty(...)

pulpdrew
pulpdrew commented Aug 13, 2026
View reviewed changes
Comment thread packages/app/src/components/ChartEditor/utils.ts
};

/** What `snippet` expands to against the variable's current selection. */
function describeVariableExpansion(

@pulpdrew pulpdrew Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this stuff has been moved to packages/app/src/components/SQLEditor/variableCompletions.tsx

pulpdrew
pulpdrew commented Aug 13, 2026
View reviewed changes
Comment thread packages/common-utils/src/core/utils.ts
* the user through `resolveRawSqlMacros` — except when there is no context at
* all, in which case they silently pass through and are reported here.
*/
function validateVariableReferences(chartConfig: RawSqlChartConfig): {

@pulpdrew pulpdrew Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to packages/common-utils/src/variables.ts

@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from b94023d to d59161d Compare August 13, 2026 16:08
@pulpdrew
pulpdrew marked this pull request as ready for review August 13, 2026 16:18
@github-actions

github-actions Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

  • Large diff: 1240 production lines changed (threshold: 1000)
  • Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Additional context: touches the query rendering engine lightly (19 lines, under the 150-line bar for Tier 4)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 22
  • Production lines changed: 1240 (+ 1619 in test files, excluded from tier calculation)
  • Branch: drew/builder-variables
  • Author: pulpdrew

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The core template engine (scanTemplateTokens, formatVariableValues, macro expansion) escapes values per output format (escapeSqlString, escapeRegexValue, escapeLuceneValue), is quote- and comment-aware in the top-level scan, and prevents value re-expansion by inserting escaped values rather than re-scanning them. substituteChartConfigVariables drops variables from its result to prevent double-substitution. No P0/P1 ship-blocker is introduced by this diff.

🟡 P2 — recommended

  • packages/app/src/components/DBEditTimeChartForm/utils.ts:160 — expandVariablesOrLeaveRaw catches every expansion error and silently returns the unexpanded config, so a malformed reference or a macro naming an undeclared variable makes the sample-events panel build filters from literal $var text with no log or user signal.
    • Fix: Log the swallowed error before falling back so the silent degradation is diagnosable.

🔵 P3 nitpicks (3)

🔵 P3 nitpicks (3)
  • packages/common-utils/src/variables.ts:133 — findBalancedParens is quote-aware but not comment-aware, so a macro argument containing a comment with an unbalanced paren such as $__filter(col /* ) */, name) terminates the argument list early and mis-parses.
    • Fix: Skip comment spans inside findBalancedParens the way the top-level scanner already does via findCommentEnd.
  • packages/common-utils/src/types.ts:1414 — adding variables to the persisted BuilderChartConfigSchema allows a runtime snapshot of variable definitions to be serialized into a saved builder tile, which would later be consumed and could drift from the dashboard's live variables.
    • Fix: Confirm the save path strips variables before persistence (or omit it from the persisted schema and attach it only to the runtime config).
  • packages/common-utils/src/variables.ts:55 — the csv format renders values unescaped (values.join(',')); builder inputs now expose this format to more surfaces, so a value spliced outside a quoted/controlled context is unescaped by design.
    • Fix: Keep the completion help text's "Not SQL-escaped" warning prominent and note the safe usage in variable-format docs.

Reviewers (12): correctness, security, adversarial, testing, maintainability, kieran-typescript, api-contract, performance, julik-frontend-races, project-standards, agent-native, learnings-researcher.

Testing gaps: Confirm the expandVariablesOrLeaveRaw catch/fallback branch in DBEditTimeChartForm/utils.ts has a unit test; confirm the Lucene macro-error path and all four formatVariableValues escaping formats (sqlstring/regex/csv/lucene) are covered in variables.test.ts.

@pulpdrew
pulpdrew requested review from a team and knudtty and removed request for a team and knudtty August 13, 2026 17:54
@pulpdrew
pulpdrew requested a review from wrn14897 August 13, 2026 17:54
@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from d59161d to e797e6b Compare August 13, 2026 21:47
@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from e797e6b to 498e5fb Compare August 13, 2026 21:53
wrn14897
wrn14897 approved these changes Aug 17, 2026
View reviewed changes

@wrn14897 wrn14897 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@pulpdrew
pulpdrew force-pushed the drew/builder-variables branch from 2e1a153 to 60025e3 Compare August 17, 2026 17:53
@pulpdrew
pulpdrew merged commit 2eedfb2 into main Aug 17, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wrn14897 wrn14897 wrn14897 approved these changes

Assignees

No one assigned

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pulpdrew @wrn14897